feat: set up new Preferences page & edit/view drawers (MSQ V2) #5654
Conversation
✅ Deploy Preview for partners-bloom-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-flagly ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-public-seeds ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-exygy-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-angelopolis ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-lakeview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for partners-bloom-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-public-seeds ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-exygy-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-angelopolis canceled.
|
✅ Deploy Preview for bloom-flagly ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for bloom-lakeview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I don't understand why the Backend Integration test keeps failing. When I run it locally, I do get one failure…but it's a totally different failure in some other file. 😅 |
jaredcwhite
changed the title
jaredcwhite
added
the
2 reviews needed
|
@jaredcwhite the failing property backend test appears to be because of an edge case depending on the order that the tests run. The test in question is not passing any parameters so it's defaulting to 10 as the page size, however some other tests add properties so sometimes there are more than 10 properties meaning one or both of the properties we are looking for might be on the second page. We will have to make the test more generic. I can quickly put up a PR to take that into account. |
ludtkemorgan
left a comment
ludtkemorgan
left a comment
There was a problem hiding this comment.
So far looking pretty good functionally. I'm still making my way through the changes but wanted to get my review out for what I have reviewed so far.
Can you also add new frontend tests for this new functionality? We should have a new test suite for the new edit page along with all of the drawers/modals. Let me know if you need any assistance with coming up with test cases for those
| type="text" | ||
| dataTestId={"preference-title"} | ||
| defaultValue={questionData?.name} | ||
| errorMessage={`${t("errors.requiredFieldError")}. ${t( |
There was a problem hiding this comment.
This error message is showing up even when I have a valid value here but there is an issue somewhere else on the form.
So for example I filled out the title but didn't add any options and clicked "save". The error for this field appeared along with the "add option" button being red. Only the "add option" error should have appeared
There was a problem hiding this comment.
I was wondering about the regex I added for this…it requires an uppercase first letter but maybe I need to relax that. I think that's why you were running into an error here.
There was a problem hiding this comment.
I also noticed the regex doesn't allow parenthesis but when you copy a preference it defaults to adding (copy) to the preference name. I wonder how we should handle that.
This is the current flow:
- click "edit" or "view" on any preference
- Click "copy"
- open the new preference and click "save" or "show to partners"
- The name field shows an error
| @@ -0,0 +1,152 @@ | |||
| import React, { useContext, useState } from "react" | |||
There was a problem hiding this comment.
nit: I'm worried having the name preferences-new as the folder name is going to be here for a long time if we never get around to cleaning up the feature flag. Could you rename the folder something like preferences-v2 to follow the pattern we have been using for the MSQ refactor?
| @@ -0,0 +1,201 @@ | |||
| import React, { useContext, useState, useMemo } from "react" | |||
There was a problem hiding this comment.
nit: same as other comment about calling this preferences-v2.tsx to follow the naming pattern we have been using for the feature flag
| readerOnly | ||
| dataTestId={"preference-option-title"} | ||
| defaultValue={optionData?.name} | ||
| errorMessage={`${t("errors.requiredFieldError")}. ${t( |
There was a problem hiding this comment.
Same here in the options drawer with the error message showing when there is a valid value but an error somewhere else on the page
| async function main() { | ||
| const { | ||
| values: { environment, jurisdictionName }, | ||
| values: { environment, jurisdictionName, msqV2 }, |
| onError?: (err: any) => void | ||
| } | ||
|
|
||
| export const useMutate = <UseMutateResponse>() => { |
There was a problem hiding this comment.
question: with this file here can we switch the usage of the ui-component version over to this version? Looks like the ui-component useMutate is used in a handful of files
| type="text" | ||
| dataTestId={"preference-title"} | ||
| defaultValue={questionData?.name} | ||
| errorMessage={`${t("errors.requiredFieldError")}. ${t( |
There was a problem hiding this comment.
I also noticed the regex doesn't allow parenthesis but when you copy a preference it defaults to adding (copy) to the preference name. I wonder how we should handle that.
This is the current flow:
- click "edit" or "view" on any preference
- Click "copy"
- open the new preference and click "save" or "show to partners"
- The name field shows an error
| </Button> | ||
| <Button type="button" variant="primary-outlined" onClick={toggleVisibility}> | ||
| {!questionData || questionData?.status === MultiselectQuestionsStatusEnum.draft | ||
| ? "Show to Partners" |
There was a problem hiding this comment.
nit: can these strings be added to the partners general.json file?
| <Button | ||
| type="button" | ||
| variant="primary" | ||
| onClick={async () => { |
There was a problem hiding this comment.
suggestion: could this onClick function declaration get pulled out of being within the <button>?
| view: { | ||
| content: ( | ||
| <Button variant="text" onClick={() => setOptionData(item)}> | ||
| View |
There was a problem hiding this comment.
nit: same here for getting it from the general.json file
| { | ||
| name: 'At least one member of my household is a city employee', | ||
| collectAddress: false, | ||
| ordinal: 0, |
There was a problem hiding this comment.
nit: the default ordinal should be 1 and not 0. Otherwise these seeded preferences show as ordinal 0
| profile?.jurisdictions.every((juris) => juris.enableGeocodingRadiusMethod) | ||
| const radiusExpand = | ||
| (optionData?.validationMethod === ValidationMethodEnum.radius && | ||
| watch("validationMethod") === undefined) || |
There was a problem hiding this comment.
suggestion: looks like you are doing watch("validationMethod") 4 times here. Could it be a variable created above all of these?
|
@ludtkemorgan Regarding tests, I'm thinking of adapting the unit/component-level tests to the new pages, and leaving Cypress alone for now. I feel like maintaining e2e tests for two different DB schemas would be really complicated, so maybe we can revisit if/how to do it once we sunset V1. |
That sounds great! Yeah, I don't think we need cypress tests at this time |
This PR addresses #5597, #5149, #5650, #5651
Description
The PR adds a new, separate screen for viewing/editing Preferences (at the path
/settings/preferences-new) which allows for managing MSQ V2 style preferences.How Can This Be Tested/Reviewed?
To seed your local DB with preferences in V2, run:
yarn setup --msqV2in theapifolderThen you can log in as a Bloomington admin in Partners and access the new page via Settings. Otherwise you'll still see the old page and seed data.
It's possible to view the two seed preferences as well as add new preferences. Unfortunately there's no easy way to flip between Edit and View for the same preference. You could try manually editing the code in the
preferences-new.tsxfor the table. I'm also not sure how easy it is to test various other statuses available in the enum.Some of the code for editing a preference is copied over and modified from the previous V1 editing, but all of the code for viewing is new, so it's possible I missed some edge cases or something in the UI could be improved. This is what I would call a "first draft".
There are also some follow-up issues which will touch on certain things in here, such as adding search, pagination, and sorting to the new preferences table.
Author Checklist:
yarn generate:clientand/or created a migration when requiredReview Process: